-
-
Notifications
You must be signed in to change notification settings - Fork 341
gix-testtools: Upgrade versions of gix crates #1510
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This avoids duplicate dependencies in packages depending on both gix and gix-testtools.
Would it be possible to get a gix-testtools release based on this? Also, is there some process that could avoid having gix-testtools get out of sync? |
Actually, I don't think I can do that as it will break the ability of I'd also love to not have this special-case, but it will probably need more time than I can invest into |
The previous commit added `gix-testtools` (by relative path) as a dev dependency of `gix-index`, so `gix_testtools::size_ok`. Because `gix-testtools` itself depends on `gix-index` -- at an earlier version to not break releasing with csr (see discussion in GitoxideLabs#1510 for general info) -- this causes `cargo`, when running in the top level workspace directory, to consider `-p gix-index` without an explicit version to be ambiguous. This made the full CI `test` job fail when the `check` recipe attempts to run `cargo check` on `gix-index`, with the message error: There are multiple `gix-index` packages in your project, and the specification `gix-index` is ambiguous. Please re-run this command with one of the following specifications: gix-index@0.33.1 gix-index@0.36.0 error: Recipe `check` failed on line 87 with exit code 101 where the line number is from the `justfile`. To fix this, this changes the command to change to the `gix-index` directory instead of passing `-p gix-index`. (This technique is used elsewhere in the same recipe already.)
The previous commit added `gix-testtools` (by relative path) as a dev dependency of `gix-index`, to use `gix_testtools::size_ok`. Because `gix-testtools` itself depends on `gix-index` -- at an earlier version to not break releasing with csr (see discussion in GitoxideLabs#1510 for general info) -- this causes `cargo`, when running in the top level workspace directory, to consider `-p gix-index` without an explicit version to be ambiguous. This made the full CI `test` job fail when the `check` recipe attempts to run `cargo check` on `gix-index`, with the message error: There are multiple `gix-index` packages in your project, and the specification `gix-index` is ambiguous. Please re-run this command with one of the following specifications: gix-index@0.33.1 gix-index@0.36.0 error: Recipe `check` failed on line 87 with exit code 101 where the line number is from the `justfile`. To fix this, this changes the command to change to the `gix-index` directory instead of passing `-p gix-index`. (This technique is used elsewhere in the same recipe already.)
As this PR is blocked by technicalities and won't budge for that reason, I am closing it as the issue is likely to persist until To re-state the problem:
The workaround for the problem as it's employed now is to let Now that I am thinking about it, another solution would be to completely remove all In theory, Thus it's probably better to wait until |
The `gix-testtools` crate depends on previous major/breaking versions of some `gix-*` crates, as described in GitoxideLabs#1510 (comment) and further discussed in GitoxideLabs#1886. This creates a situation where `gix-testtools` will sometimes use `gix-*` crates in vulnerable versions. Even as `gix-testtools` is used in this project, that could in principle cause a problem for some vulnerabilities. So it is correct in general to consider vulnerable `gix-testtools` dependencies significant. However, in most vulnerabilities so far, the specific use in `gix-testtools` as part of gitoxide's test suite has been acceptable. (Other common uses of `gix-testtools`, if they are in test suites operating on trusted data as here, may be in a similar situation, but it may not be reasonable to assume that broadly.) When `cargo deny advisories` fails on CI due to a `gix-testtools` dependency on an old version of a `gix-*` crate, it makes it harder to notice if *other* vulnerable dependencies are also being used. A usual workaround for this would be to add the vulnerability's RUSTSEC ID to the `ignore` list in `deny.toml`, but that would weaken the operation of `cargo deny` far too much, because: - The distraction here is mainly, or perhaps only, a problem in CI, so no change to `deny.toml` may be needed. - It should remain easy to run `cargo deny` in such a way that the dependence of `gix-testtools` on vulnerable crate versions is revealed, and it should be obvious from the command that is run whether that information would be shown or not. - The advisories themselves should not be ignored because they are unexpected, and potentially highly consequently, if they arise from any other crate. - It is useful to be able to easily compare the output of `cargo deny advisories` with and without such messages. So this multiplies the step into two, running `cargo deny` twice for advisories: 1. Initially including dependencies through `gix-testtools`, but marking the step as `continue-on-error: true` so it doesn't fail the job. 2. Again without dependencies through `gix-testtools`, allowing the step to fail the job on vulnerabilities found via other crates.
This avoids duplicate dependencies in packages depending on both gix and
gix-testtools.